Skip to content

Add relative_path built-in function#3185

Open
hunter-hongg wants to merge 1 commit intocasey:masterfrom
hunter-hongg:master
Open

Add relative_path built-in function#3185
hunter-hongg wants to merge 1 commit intocasey:masterfrom
hunter-hongg:master

Conversation

@hunter-hongg
Copy link
Copy Markdown

Adds a new built-in function relative_path that computes a relative path from one path to another using Path::strip_prefix. This provides an OS-agnostic alternative to the relpath bash utility, which breaks on macOS.

Adds a new built-in function `relative_path` that computes a relative
path from one path to another using `Path::strip_prefix`. This provides
an OS-agnostic alternative to the `relpath` bash utility, which breaks
on macOS where bash runs as zsh.

The function handles both absolute and relative paths across all
supported platforms.
}

fn relpath(_context: Context, target: &str, base: &str) -> FunctionResult {
let base = match Utf8Path::new(base).canonicalize(){
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of canonicalize unless necessary. I would join them both to the current working directory, lexiclean them, and then compute the relative path.

Ok(t) => t,
Err(e) => return Err(format!("Canonicalize path {target} failed: {e}")),
};
if let Some(rel) = pathdiff::diff_paths(&target, &base) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer not to add a dependency for a single simple function.

if let Some(rel) = pathdiff::diff_paths(&target, &base) {
Ok(rel.display().to_string())
} else {
Ok(target.display().to_string()) // fallback to absolute path
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me that it's useful to fall back to an absolute path. Is this desirable? Would an error be better?

/test-utilities/Cargo.lock
/test-utilities/target
/tmp
/3104issue.json
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/3104issue.json

lexiclean = "0.0.1"
libc = "0.2.0"
num_cpus = "1.15.0"
pathdiff = "0.2.3"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pathdiff = "0.2.3"

I'd rather not add a dependency for a single function, it should just be implemented in this PR.

@casey
Copy link
Copy Markdown
Owner

casey commented Mar 30, 2026

Also, I couldn't find which relpath you're referring to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants